feature(otel-collector): ordered processors list#910
Conversation
filiphaftek
changed the title
| {{- if .Values.openTelemetry.gateway.config.traces.processorOrderedList }} | ||
| {{- range .Values.openTelemetry.gateway.config.traces.processorOrderedList }} | ||
| - {{ . }} | ||
| {{- end }} | ||
| {{- else }} | ||
| {{- range $key, $val := .Values.openTelemetry.gateway.config.traces.processors }} | ||
| - {{ $key }} | ||
| {{- end }} | ||
| {{- end }} |
There was a problem hiding this comment.
Should processors and processorOrderedList be mutually exclusive? Curious why we shouldn't deprecate processors or simply change its shape as processors is effectively buggy it seems
There was a problem hiding this comment.
@bobheadxi currently processorOrderedList only sets the order, not defines the processors. If you think having them separated is better, we need to make list with full definitions, similar to https://sourcegraph.sourcegraph.com/r/github.com/sourcegraph/deploy-sourcegraph-helm/-/blob/charts/sourcegraph/values.yaml?L667 - if you think this is more clear, I can refactor?
| {{- if .Values.openTelemetry.gateway.config.traces.processors }} | ||
| processors: | ||
| {{- if .Values.openTelemetry.gateway.config.traces.processorOrderedList }} |
There was a problem hiding this comment.
doesn't this mean without processors, processorOrderedList does not work?
There was a problem hiding this comment.
yes, processorOrderedList is just addition to processors, not replacement, pls see above comment/
3 participants
otel-collector processors list is using map keys, which is sorted alphabethically.
Some processoers should be used in order i.e. tail_sampling before batch, so this PR adds additional property to set proper order of the processors. It can only be used together with processors definition.
Change is backward compatible
Checklist
Test plan
Unit tests.
e2e